Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add workaround allowing resubmission #241

Merged
merged 2 commits into from
Nov 19, 2020

Conversation

r0ps3c
Copy link
Contributor

@r0ps3c r0ps3c commented Nov 13, 2020

As the commit/other information doesn't change when performing a
rebuild, resubmissions will fail with an HTTP 422 error. This adds a
workaround for such cases, appending a random value to the id and
attempting a resubmission with that.

As the commit/other information doesn't change when performing a
rebuild, resubmissions will fail with an HTTP 422 error. This adds a
workaround for such cases, appending a random value to the id and
attempting a resubmission with that.
@r0ps3c
Copy link
Contributor Author

r0ps3c commented Nov 13, 2020

This should fix #238

@r0ps3c r0ps3c closed this Nov 19, 2020
@r0ps3c r0ps3c reopened this Nov 19, 2020
@r0ps3c
Copy link
Contributor Author

r0ps3c commented Nov 19, 2020

@TheKevJames - feedback on this would be welcome. Not sure why ci/circleci is failing but otherwise looks ready for review to me.

Copy link
Owner

@TheKevJames TheKevJames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me -- thanks for providing a fix for this (and apologies for taking so long to check out this PR!).

@TheKevJames TheKevJames merged commit 0de0c01 into TheKevJames:master Nov 19, 2020
# failed due to resubmission (non-unique)
if response.status_code == 422:
self.config['service_job_id'] = '{}-{}'.format(
self.config['service_job_id'], random.randint(0, sys.maxsize))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this lead to a potential KeyError on GitHub Actions?
Because the service_job_id is only set if job is set in Coveralls.__init__():
https://github.com/coveralls-clients/coveralls-python/blob/2.2.0/coveralls/api.py#L56-L57
However job is always None in Coveralls.load_config_from_github():
https://github.com/coveralls-clients/coveralls-python/blob/2.2.0/coveralls/api.py#L92-L109

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may be right - looks like github actions are one of/the only ones that requires use of a token, and then submits this using the second method documented at https://docs.coveralls.io/api-introduction. Have you seen this occur in practice?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think I saw it happening with when using service_name=github and getting a 422.
The fix for the KeyError was to simply provide service_job_id a value (None in my case) to the Coveralls constructor. That was enough for me. See below for the fix including regression tests.
AndreMiras/coveralls-python-action@50af55e
Since it could well be me misusing the package, I guess we can simply acknowledge this bug/feature and take no actions until someone else complains

andy-maier pushed a commit to andy-maier/coveralls-python that referenced this pull request Dec 23, 2022
As the commit/other information doesn't change when performing a
rebuild, resubmissions will fail with an HTTP 422 error. This adds a
workaround for such cases, appending a random value to the id and
attempting a resubmission with that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants